Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't ask what edition we are in; ask what edition a span is in #50999

Conversation

nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented May 23, 2018

We now track the edition of each span. Using that info when gating the Rust 2018 interpretation means that macros from Rust 2015 crates "just work" when used in Rust 2018 crates (at least in the case of use paths).

Fixes #50172

r? @petrochenkov

cc @Manishearth @alexcrichton

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 23, 2018
We now track the edition of each span. Using that info when gating the
Rust 2018 interpretation means that macros from Rust 2015 crates "just
work" when used in Rust 2018 crates (at least in the case of `use`
paths).
@nikomatsakis nikomatsakis force-pushed the issue-50172-macro-edition-hygiene branch from e32d992 to 24529d9 Compare May 23, 2018 16:51
@Manishearth
Copy link
Member

We now track the edition of each span.

oh, my.

@nikomatsakis
Copy link
Contributor Author

Hmm, I'm not sure this is behaving right in some of the corner cases. I'll come back to it in a bit. But for example, when I add a macro to the Rust 2015 helper like this:

#[macro_export]
macro_rules! print_me {
    ($p:path) => {
        {
            use $p as V;
            println!("{}", V);
        }
    }
}

I expect the paths supplied to the macro from the Rust 2018 crate to use Rust 2018 semantics. I'm not 100% sure if they are or not yet :p I have to dig into the output a bit deeper.

@nikomatsakis
Copy link
Contributor Author

(Obviously once you start concatenating paths and things it becomes less clear what is right -- I wanted to add some tests where we build paths like $a :: $b (where those are idents) as well -- it may be that we should be using the span of the use or something to decide, though in my example above it feels like we want the span of the path as a whole.)

@alexcrichton
Copy link
Member

🗻

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:43:26] ....................................................................................................
[00:43:31] ...........................................................................i........................
[00:43:36] ....................................................i...............................................
[00:43:40] ........................................................................ii..........................
[00:43:45] ....F.F.............................................................................................
[00:43:53] iiiii...................................................
[00:43:53] failures:
[00:43:53] 
[00:43:53] 
[00:43:53] ---- [ui] ui/rust-2018/inject-2015-use-root-module-path.rs stdout ----
[00:43:53] normalized stderr:
[00:43:53] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:43:53] error[E0432]: unresolved import `x::y`
[00:43:53]   --> $DIR/inject-2015-use-root-module-path.rs:28:5
[00:43:53]    |
[00:43:53] LL |     print_me!(x::y); //~ ERROR unresolved import `x::y`
[00:43:53]    |     ^^^^^^^^^^^^^^^^ no `y` in `x`
[00:43:53]    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
[00:43:53] 
[00:43:53] error: aborting due to previous error
[00:43:53] 
[00:43:53] 
[00:43:53] For more information about this error, try `rustc --explain E0432`.
[00:43:53] 
[00:43:53] 
[00:43:53] 
[00:43:53] The actual stderr differed from the expected stderr.
[00:43:53] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rust-2018/inject-2015-use-root-module-path/inject-2015-use-root-module-path.stderr
[00:43:53] To update references, rerun the tests and pass the `--bless` flag
[00:43:53] To only update this specific test, also pass `--test-args rust-2018/inject-2015-use-root-module-path.rs`
[00:43:53] error: 1 errors occurred comparing output.
[00:43:53] status: exit code: 101
[00:43:53] status: exit code: 101
[00:43:53] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/rust-2018/inject-2015-use-root-module-path.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rust-2018/inject-2015-use-root-module-path/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--edition" "2018" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rust-2018/inject-2015-use-root-module-path/auxiliary" "-A" "unused"
[00:43:53] ------------------------------------------
[00:43:53] 
[00:43:53] ------------------------------------------
[00:43:53] stderr:
[00:43:53] stderr:
[00:43:53] ------------------------------------------
[00:43:53] {"message":"unresolved import `x::y`","code":{"code":"E0432","explanation":"\nAn import was unresolved.\n\nErroneous code example:\n\n```compile_fail,E0432\nuse something::Foo; // error: unresolved import `something::Foo`.\n```\n\nPaths in `use` statements are relative to the crate root. To import items\nrelative to the current and parent modules, use the `self::` and `super::`\nprefixes, respectively. Also verify that you didn't misspell the import\nname and that the import exists in the module from where you tried to\nimport it. Example:\n\n```\nuse self::something::Foo; // ok!\n\nmod something {\n    pub struct Foo;\n}\n# fn main() {}\n```\n\nOr, if you tried to use a module from an external crate, you may have missed\nthe `extern crate` declaration (which is usually placed in the crate root):\n\n```\nextern crate core; // Required to use the `core` crate\n\nuse core::any;\n# fn main() {}\n```\n"},"level":"error","spans":[{"file_name":"<print_me macros>","byte_start":26,"byte_end":34,"line_start":1,"line_end":1,"column_start":27,"column_end":35,"is_primary":true,"text":[{"text":"( $ p : path ) => { { use $ p as V ; println ! ( \"{}\" , V ) ; } }","highlight_start":27,"highlight_end":35}],"label":"no `y` in `x`","suggested_replacement":null,"suggestion_applicability":null,"expansion":{"span":{"file_name":"/checkout/src/test/ui/rust-2018/inject-2015-use-root-module-path.rs","byte_start":1024,"byte_end":1040,"line_start":28,"line_end":28,"column_start":5,"column_end":21,"is_primary":false,"text":[{"text":"    print_me!(x::y); //~ ERROR unresolved import `x::y`","highlight_start":5,"highlight_end":21}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null},"macro_decl_name":"print_me!","def_site_span":{"file_name":"<print_me macros>","byte_start":0,"byte_end":65,"line_start":1,"line_end":1,"column_start":1,"column_end":66,"is_primary":false,"text":[{"text":"( $ p : path ) => { { use $ p as V ; println ! ( \"{}\" , V ) ; } }","highlight_start":1,"highlight_end":66}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}}}],"children":[],"rendered":"error[E0432]: unresolved import `x::y`\n  --> /checkout/src/test/ui/rust-2018/inject-2015-use-root-module-path.rs:28:5\n   |\nLL |     print_me!(x::y); //~ ERROR unresolved import `x::y`\n   |     ^^^^^^^^^^^^^^^^ no `y` in `x`\n   |\n   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)\n\n"}
[00:43:53] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:43:53] {"message":"For more information about this error, try `rustc --explain E0432`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0432`.\n"}
[00:43:53] ------------------------------------------
[00:43:53] 
[00:43:53] 
[00:43:53] thread '[ui] ui/rust-2018/inject-2015-use-root-module-path.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3044:9
[00:43:53] 
[00:43:53] 
[00:43:53] ---- [ui] ui/rust-2018/inject-2015-use-root-module.rs stdout ----
[00:43:53] 
[00:43:53] error: test compilation failed although it shouldn't!
[00:43:53] status: exit code: 101
[00:43:53] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/rust-2018/inject-2015-use-root-module.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rust-2018/inject-2015-use-root-module/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--edition" "2018" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rust-2018/inject-2015-use-root-module/auxiliary" "-A" "unused"
[00:43:53] ------------------------------------------
[00:43:53] 
[00:43:53] ------------------------------------------
[00:43:53] stderr:
[00:43:53] stderr:
[00:43:53] ------------------------------------------
[00:43:53] {"message":"unresolved import `crate::x::y`","code":{"code":"E0432","explanation":"\nAn import was unresolved.\n\nErroneous code example:\n\n```compile_fail,E0432\nuse something::Foo; // error: unresolved import `something::Foo`.\n```\n\nPaths in `use` statements are relative to the crate root. To import items\nrelative to the current and parent modules, use the `self::` and `super::`\nprefixes, respectively. Also verify that you didn't misspell the import\nname and that the import exists in the module from where you tried to\nimport it. Example:\n\n```\nuse self::something::Foo; // ok!\n\nmod something {\n    pub struct Foo;\n}\n# fn main() {}\n```\n\nOr, if you tried to use a module from an external crate, you may have missed\nthe `extern crate` declaration (which is usually placed in the crate root):\n\n```\nextern crate core; // Required to use the `core` crate\n\nuse core::any;\n# fn main() {}\n```\n"},"level":"error","spans":[{"file_name":"<print_me macros>","byte_start":26,"byte_end":34,"line_start":1,"line_end":1,"column_start":27,"column_end":35,"is_primary":true,"text":[{"text":"( $ p : path ) => { { use $ p as V ; println ! ( \"{}\" , V ) ; } }","highlight_start":27,"highlight_end":35}],"label":"no `y` in `x`","suggested_replacement":null,"suggestion_applicability":null,"expansion":{"span":{"file_name":"/checkout/src/test/ui/rust-2018/inject-2015-use-root-module.rs","byte_start":960,"byte_end":983,"line_start":28,"line_end":28,"column_start":5,"column_end":28,"is_primary":false,"text":[{"text":"    print_me!(crate::x::y);","highlight_start":5,"highlight_end":28}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null},"macro_decl_name":"print_me!","def_site_span":{"file_name":"<print_me macros>","byte_start":0,"byte_end":65,"line_start":1,"line_end":1,"column_start":1,"column_end":66,"is_primary":false,"text":[{"text":"( $ p : path ) => { { use $ p as V ; println ! ( \"{}\" , V ) ; } }","highlight_start":1,"highlight_end":66}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}}}],"children":[],"rendered":"error[E0432]: unresolved import `crate::x::y`\n  --> /checkout/src/test/ui/rust-2018/inject-2015-use-root-module.rs:28:5\n   |\nLL |     print_me!(crate::x::y);\n   |     ^^^^^^^^^^^^^^^^^^^^^^^ no `y` in `x`\n   |\n   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)\n\n"}
[00:43:53] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:43:53] {"message":"For more information about this error, try `rustc --explain E0432`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0432`.\n"}
[00:43:53] ------------------------------------------
[00:43:53] 
[00:43:53] 
[00:43:53] thread '[ui] ui/rust-2018/inject-2015-use-root-module.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3044:9
[00:43:53] 
[00:43:53] failures:
[00:43:53] failures:
[00:43:53]     [ui] ui/rust-2018/inject-2015-use-root-module-path.rs
[00:43:53]     [ui] ui/rust-2018/inject-2015-use-root-module.rs
[00:43:53] test result: FAILED. 1438 passed; 2 failed; 16 ignored; 0 measured; 0 filtered out
[00:43:53] 
[00:43:53] 
[00:43:53] 
[00:43:53] 
[00:43:53] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:43:53] 
[00:43:53] 
[00:43:53] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:43:53] Build completed unsuccessfully in 0:02:25
[00:43:53] Build completed unsuccessfully in 0:02:25
[00:43:53] make: *** [check] Error 1
[00:43:53] Makefile:58: recipe for target 'check' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0a7e6e2d
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

/// particular span: that way, when you are looking at code
/// creating a macro from a Rust 2015 crate, you will use the Rust
/// 2015 Edition rules.
pub fn local_edition(&self) -> Edition {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably just implement Default for Edition.
"Local edition" needs to be kept in global data anyway because it's an "edition of SyntaxContext(0) aka not-a-macro-expansion" and it's set from default-default Edition2015 to self.opts.edition immediately after options are parsed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. True, though I'm not wild about the fact that we rely so heavily on global / TLS data for this stuff.

@petrochenkov
Copy link
Contributor

@nikomatsakis

I expect the paths supplied to the macro from the Rust 2018 crate to use Rust 2018 semantics.

Paths currently have spans from the "concatenation context" (see fn parse_path_common) just because they give better diagnostics.

In general, we can strictly rely only on spans of "atomic" tokens, for multi-token things like paths there are always multiple choices (#50122).
This is an important topic, and for imports it's especially important because we need to decide in what context the crate root will be resolved. See #50376 that's very similar to the problem you are trying to solve.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2018
@bors
Copy link
Contributor

bors commented May 28, 2018

☔ The latest upstream changes (presumably #48309) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis
Copy link
Contributor Author

@petrochenkov

Sorry for the delay, got caught up in other things. I agree that #50122 and #50376 are intimately related. I'm not 100% sure I would even draw a distinction between them. I'll ponder a bit and try to leave a comment over there in those issues.

@TimNN TimNN added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 5, 2018
@pietroalbini pietroalbini added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Jun 25, 2018
@nrc nrc added the A-edition-2018-lints Area: Lints supporting the 2018 edition label Jul 2, 2018
@nrc nrc mentioned this pull request Jul 3, 2018
@pietroalbini pietroalbini added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Jul 9, 2018
@alexcrichton alexcrichton added this to the Rust 2018 Preview 2 milestone Jul 11, 2018
@alexcrichton
Copy link
Member

I've written up a comment about a recent chat @nikomatsakis and I had where the conclusion was to basically punt on this PR for now. It's not clear whether this is the right strategy amongst alternatives, so we're going to not worry about solving this for the edition (as it doesn't seem all that pressing) and fix it if we uncover it later.

As a result, I'm removing the milestone from this PR.

@alexcrichton alexcrichton removed this from the Rust 2018 RC milestone Aug 24, 2018
@alexcrichton alexcrichton added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Aug 24, 2018
@TimNN
Copy link
Contributor

TimNN commented Aug 28, 2018

[...] where the conclusion was to basically punt on this PR for now.

Does this mean this PR can just be closed for now?

@alexcrichton
Copy link
Member

Heh if I recall right @nikomatsakis felt bad closing the PR himself so I mentioned we can let normal triage processes close the PR. I'll perhaps preempt that slightly by closing it myself...

@nikomatsakis
Copy link
Contributor Author

I'm of two minds because I do still want to see this reach some conclusion =) but keeping this PR open serves no real purpose.

bors added a commit that referenced this pull request Nov 25, 2018
[beta] resolve: Implement edition hygiene for imports and absolute paths

The changes in behavior of imports and absolute paths are the most significant breaking changes of 2018 edition.
However, these changes are not covered by edition hygiene, so macros defined by 2015 edition crates expanded in 2018 edition crates are still interpreted in the 2018 edition way when they contain imports or absolute paths.
This means the promise of seamless integration of crates built with different editions, including use of macros, doesn't hold fully.
This PR fixes that and implements edition hygiene for imports and absolute paths, so they behave according to the edition in which they were written, even in macros.

### Detailed rules (mostly taken from #50376)
#### Preface
We keep edition data per-span in the compiler. This means each span knows its edition.
Each identifier has a span, so each identifier knows its edition.

#### Absolute paths

Explicitly written absolute paths `::ident::...` are desugared into something like `{{root}}::ident::...` in the compiler, where `{{root}}` is also treated as an identifier.
`{{root}}` inherits its span/hygienic-context from the token `::`.

If the span is from 2015 edition, then `{{root}}` is interpreted as the current crate root (`crate::...` with same hygienic context).
If the span is from 2018 edition, then `{{root}}` is interpreted as "crate universe" (`extern::...`).

#### Imports

To resolve an import `use ident::...` we need to resolve `ident` first.
The idea in this PR is that `ident` fully knows how to resolve itself.

If `ident`'s span is from 2015 edition, then the identifier is resolved in the current crate root (effectively `crate::ident::...` where `crate` has the same hygienic context as `ident`).
If `ident`'s span is from 2018 edition, then the identifier is resolved in the current scope, without prepending anything (well, at least with uniform paths).

There's one corner case in which there's no identifier - prefix-less glob import `use *;`.
In this case the span is inherited from the token `*`.
`use *;` && `is_2015(span(*))` -> `use crate::*;` && `span(crate) == span(*)`.
`use *;` && `is_2018(span(*))` -> error.

---
Why beta:
	- Compatibility of 2015 edition crates with 2018 edition crates, including macros, is one of the main promises of the edition initiative.
	- This is technically a breaking change for 2018 edition crates or crates depending on 2018 edition crates.
	- ~This PR is based on #55884 which hasn't landed on nightly yet :)~ No longer true (#56042).

Previous attempt #50999
Closes #50376
Closes #52848
Closes #53007

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2018-lints Area: Lints supporting the 2018 edition S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants